Skip to content

Conversation

@ErwanBeurienne
Copy link

@ErwanBeurienne ErwanBeurienne commented Dec 17, 2025

Hello !

In this pull request, I add a new helper function, adapt_P_and_calib_params(), to handle cases where the number of cameras defined in the calibration files exceeds the number of pose folders available. This function issues clear warnings and automatically adapts the calibration parameters so that only the relevant cameras are used.

The main changes are:

  1. Add adapt_P_and_calib_params() to centralize the logic for aligning calibration parameters with available pose folders.
  2. Update personAssociation.py and triangulation.py to call this function when loading calibration data.
  3. Improve logging messages to make it explicit when some cameras from the calibration files are ignored because they have no corresponding pose folder.

This should make the pipeline more robust to inconsistent calibration/pose configurations while preserving existing behavior when the inputs are consistent.

Added adapt_P_and_calib_params to handle cases where the number of cameras in calibration files exceeds the number of pose folders, issuing warnings and adapting parameters accordingly. Updated personAssociation.py and triangulation.py to use this logic and provide clearer logging messages.
@davidpagnon
Copy link
Collaborator

Thanks, Erwan!
I'll probably have a look at it next week :)

fix: avoid calib/pose camera mismatch with non-contiguous IDs

The previous index-based alignment could assign wrong calibration parameters
when cameras in calib file are non-contiguous (e.g. cam1, cam2, cam4, cam6).

Remove adapt_P_and_calib_params. Handle pose-dir filtering directly in
retrieve_calib_params() and computeP() via json_pose_dirs_names, and update
personAssociation.py and triangulation.py accordingly.
@ErwanBeurienne
Copy link
Author

I pushed a follow-up fix: the previous approach could mismatch camera parameters when the calib file contains non-contiguous camera IDs (e.g. cam1, cam2, cam4, cam6).

The filtering is now done by camera name (from calib file) and json pose folder directly in retrieve_calib_params() and computeP(). adapt_P_and_calib_params() was removed.

@davidpagnon
Copy link
Collaborator

Hi Erwan,

Let me know if I misunderstood anything:

  • In common.py, you added the optional parameter json_pose_dirs_names in retrieve_calib_params and computeP, and used it in personAssociation.py and triangulation.py.
  • If the parameter is not used, the behavior is unchanged. Otherwise, it associates cameras in the calibration file with those in the _jons pose files based on their names

This looks good to me, however could you address the couple of questions I asked in the review?

Thanks, and I wish you a great new year's eve!

Copy link
Author

@ErwanBeurienne ErwanBeurienne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To handle misalignment between calibration files and pose folders (e.g., non-contiguous or missing camera IDs), the following changes were implemented:

  • In common.py, the optional parameter json_pose_dirs_names was added to retrieve_calib_params and computeP, and used in personAssociation.py and triangulation.py.
  • If the parameter is not provided, the behavior remains unchanged. When used, it associates cameras by name between calibration files and _json pose folders.

@ErwanBeurienne
Copy link
Author

Hi David!

Your understanding is correct. I addressed your questions in the PR comments. If "the review" refers to a specific section or format I missed, let me know where to add or move the information.

@ErwanBeurienne
Copy link
Author

And of course, I wish you a Happy New Year!
I just checked the PR and realized I forgot it!

@davidpagnon
Copy link
Collaborator

davidpagnon commented Jan 8, 2026

Lol no problem! I truly appreciate your small iterative pull requests, I'm sorry it takes time to review them but I'm not forgetting them!
And happy New Year to you, too :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants